fix: green the test suite — 2 real product bugs, 13 stale tests, no CI exclusions - #253

What

The 15 pre-existing failures excluded from CI in #251 are now fixed. Zero --deselect remain.

Triage was per-test: is this a stale test, or is it catching a real bug? Two were real.

Real product bugs (2)

1. provider add silently did nothing when a stale key sat in keys.env

_claimed_env_vars() unioned in KeyManager().stored_keys(), so any name present in ~/.amplifier/keys.env counted as "claimed" for collision detection. Since provider remove deliberately leaves the key behind (design §8 risk 6), and provider add itself writes it, the first instance of a provider type collided with a credential nobody owned.

Reproduced end-to-end with the real CLI, clean HOME, one stale OPENAI_API_KEY= line in keys.env:

BEFORE (main @ ee19d4a):
  provider-openai already uses OPENAI_API_KEY (instance 'an existing instance').
  This instance needs its own credential source.
  Env var for this instance's key (OPENAI_PROVIDER_OPENAI_API_KEY): Cancelled.

AFTER (this branch):
  Configuring OpenAI API Key (Found in environment/keyring - will use if you don't
  configure) Enter your OpenAI API key (press Enter to keep existing) ()

A collision announced against an instance that does not exist, a garbage per-instance var name derived from the module id, and on EOF/non-tty it exits 0 having added nothing. It also made design §5.4.4 (stale-credential warn-and-reuse) unreachable dead code, because claimed ⊇ stored_keys() meant has_stored_key(chosen_name) could never be true in production.

Fix: split the two notions. _config_claimed_env_vars() (an instance actually references the name via ${VAR}) drives collision detection; _claimed_env_vars() (+ keys.env) stays as-is for normalize_provider_secrets, where overwriting a saved secret is the real hazard.

2. spawn_sub_session() mutated the parent session's live config

child_agents = merged_config.setdefault("agents", {})   # parent's own dict

merge_configs() deep-copies the agents dict only when non-empty, and merge_agent_dicts() starts from a shallow parent.copy() — so an empty parent agents dict arrived here as the parent's own object. setdefault()-then-mutate wrote the live agent registry straight into the running parent's config and handed the child the same dict. Observed:

parent.config['agents'] AFTER spawn: {'mode_agent': {'module': 'agent-x'}}
same object as pre-spawn dict: True
child agents is parent agents:  True

Empty today, populated tomorrow. Fix: build a fresh dict and rebind.

Stale tests (13)

Patch targets repointed; prompt expectations updated to the current exact string (kept as ==, not loosened to a substring check).

Non-vacuousness verified — each fixed test was confirmed to still fail when the product is broken: reintroduced the pre-#256 render-suppression gate (render tests failed), reintroduced the duplicate SESSION_END emit (assert 2 == 1), swapped CLEANUP_RENDER_END/CLEANUP_STORE_BEGIN order (ordering test failed, unrelated ones correctly still passed), reverted the load_skill f-string (both prompt tests failed).

4 new regression tests were added that fail against the product at ee19d4a — so the two real bugs cannot silently return.

CI

• Every --deselect removed. The full suite runs on every push and PR, both platforms.
• New integration job. pyproject.toml sets addopts = -m "not integration", so the 13 tests that fork a real pty and probe real termios state were skipped by default and ran nowhere — including in fix: probe tty fd pollability instead of platform-checking; add CI (Linux + macOS) #251's new CI. They are exactly the tests guarding the dedicated-tty-input mechanism, so they now get their own job on ubuntu + macOS.

uv run pytest -q              1282 passed, 1 skipped, 13 deselected, 1 xfailed
uv run pytest -m integration  13 passed

(main @ ee19d4a for comparison: 15 failed, 1259 passed)

Known follow-up, NOT fixed here

The #233 agent-propagation block in session_spawner.py silently overrides an agent's declared agents policy: merge_configs() honors agents: "none" by setting result["agents"] = {}, then the propagation loop re-adds every live coordinator agent — re-enabling delegation the agent explicitly disabled. Same for list-form allowlists. Observed: child config {'mode_agent': {'module': 'agent-x'}} for an agent declaring agents: "none".

Left out deliberately — it is a delegation-semantics change needing its own verification, not a test fix. The fix is small (gate propagation on agent_config.get("agents") being absent or "all"). Recorded here because this repo has issues disabled.

Follow-up to #247, #250, #251.